Skip to content

fix(ec2): include supported usage classes in DescribeInstanceTypes - #3347

Merged
pgermosen merged 3 commits into
floci-io:mainfrom
luiseterc:fix/describe-instance-types-usage-classes
Sep 10, 2026
Merged

fix(ec2): include supported usage classes in DescribeInstanceTypes#3347
pgermosen merged 3 commits into
floci-io:mainfrom
luiseterc:fix/describe-instance-types-usage-classes

Conversation

@luiseterc

@luiseterc luiseterc commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes the EC2 DescribeInstanceTypes response so it includes the supported usage classes required by Karpenter and other AWS-compatible clients. Fixes #3346.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

Floci previously omitted supportedUsageClasses from DescribeInstanceTypes. Karpenter 1.8.8 interprets the missing field as no compatible capacity types and rejects on-demand NodeClaims before CreateFleet; the same direct CreateFleet request succeeds. The catalog now returns on-demand and spot, and the query handler serializes both values using the AWS EC2 Query response shape.

The change covers m5.large, t4g.medium, and m6gd.large, the instance types exercised by the compatibility gate.

Changes

  • Add validated supportedUsageClasses data to the instance-type catalog, defaulting to on-demand and spot.
  • Serialize the list as <supportedUsageClasses><item>...</item></supportedUsageClasses> in DescribeInstanceTypes.
  • Add an SDK integration test asserting the values for representative amd64 and arm64 instance types.

Validation

  • ./mvnw -q -DskipTests package
  • ./mvnw -q -Dtest=Ec2IntegrationTest test

Checklist

  • ./mvnw test passes locally (targeted EC2 integration test and package build pass; full suite not run)
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@greptile-apps

greptile-apps Bot commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Greptile Summary

Adds AWS-compatible supported usage classes to EC2 DescribeInstanceTypes.

  • Defaults catalog entries to on-demand and spot and validates the field.
  • Projects the values into instance-type response maps and serializes the EC2 Query XML list.
  • Adds AWS SDK compatibility coverage for representative amd64 and arm64 instance types.
  • Includes an unrelated Step Functions test-comment clarification with no behavioral effect.
  • The required greptile_confidence label could not be added because GitHub CLI authentication is unavailable.

Confidence Score: 5/5

The PR appears safe to merge with no outstanding correctness, compatibility, or repository-rule issues.

Catalog defaults survive YAML deserialization, every described instance type receives the projected values, the XML structure matches the AWS Query list shape, and SDK compatibility coverage verifies the response.

Important Files Changed
Filename Overview
src/main/java/io/github/hectorvent/floci/services/ec2/Ec2InstanceTypeCatalog.java Adds validated default usage-class metadata and includes it in instance-type response projections.
src/main/java/io/github/hectorvent/floci/services/ec2/Ec2QueryHandler.java Emits supported usage classes using the AWS EC2 Query list shape.
compatibility-tests/sdk-test-java/src/test/java/com/floci/test/Ec2Tests.java Verifies AWS SDK decoding for three representative instance types.
src/test/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutorFailStateErrorCauseTest.java Clarifies fixture documentation without changing test behavior.

Reviews (8): Last reviewed commit: "test(ec2): document usage class regressi..." | Re-trigger Greptile

@hectorvent

hectorvent commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

@luiseterc main is broken at the moment. PR #3345 will fix it, you can rebase main as soon this PR get merged.

@luiseterc
luiseterc force-pushed the fix/describe-instance-types-usage-classes branch from c5e402c to de3920a Compare September 10, 2026 05:29
@luiseterc luiseterc changed the title ec2: include supported usage classes in DescribeInstanceTypes fix(ec2): include supported usage classes in DescribeInstanceTypes Sep 10, 2026
@luiseterc
luiseterc force-pushed the fix/describe-instance-types-usage-classes branch 3 times, most recently from b7c7f19 to aaab2cf Compare September 10, 2026 06:15
@luiseterc

luiseterc commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

@hectorvent The current Build and Test failure is isolated to shard 3 and is unrelated to this EC2 PR. CloudTrailLogWriterBoundedRetryIntegrationTest.retryStateStaysBoundedAndDeliversRetainedPrefixAfterDestinationRecovery fails at line 85 because it expects events/0000.txt but receives events/1000.txt. The test retains 1,024 records across two S3 log objects, while the object names include random suffixes; deliveredRecords() concatenates records in S3 listing order and therefore assumes an ordering the keys do not guarantee. Shards 1, 2, and 4, native builds, compatibility tests, and repository checks all pass. This looks like a CloudTrail test-ordering flake from the existing implementation in #3298, not a regression from #3347.

@hectorvent hectorvent added bug Something isn't working ec2 Amazon Elastic Compute Cloud (EC2) labels Sep 10, 2026
@luiseterc
luiseterc force-pushed the fix/describe-instance-types-usage-classes branch from aaab2cf to 4524a8b Compare September 10, 2026 17:29
pgermosen
pgermosen approved these changes Sep 10, 2026
View reviewed changes

@pgermosen pgermosen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked this against the live InstanceTypeInfo docs and the values match exactly. I also didn't just trust that the on-demand/spot default was fine for the three types your test covers, I pulled the actual instance-type-catalog.yaml and checked all 15 entries in it, none are from a Capacity-Block-eligible family, so the default is accurate across the whole catalog as it stands today, not just the tested cases. Worth keeping in mind if a p5/trn-style type is ever added later without an explicit override, but that's not a problem here.

One small thing, there's an unrelated commit bundled in here (the stepfunctions test Javadoc comment) that isn't related to the EC2 fix. Harmless since it's comment-only, just flagging it since it'll look a little odd in the squashed history.

@pgermosen
pgermosen merged commit 19f410d into floci-io:main Sep 10, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pgermosen pgermosen pgermosen approved these changes
@hectorvent hectorvent Awaiting requested review from hectorvent hectorvent is a code owner

Assignees

No one assigned

Labels

bug Something isn't working ec2 Amazon Elastic Compute Cloud (EC2)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] EC2 DescribeInstanceTypes omits supportedUsageClasses

3 participants

@luiseterc @hectorvent @pgermosen